Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

owner: fix checkpoint ts calculate in owner #163

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

amyangfei
Copy link
Contributor

What problem does this PR solve?

When the StartTs of a changefeed is smaller than any none-system table DDL, we should have a right way to calculate global resolved ts and global checkpoint ts

What is changed and how it works?

  • The global resolved ts can use the ddl resolved ts value
  • Set the global checkpoint ts to the local checkpoint cache stored in ChangeFeedInfo

Check List

Tests

  • Unit test
  • Integration test

@amyangfei amyangfei added the status/ptal Could you please take a look? label Dec 9, 2019
@@ -494,18 +493,7 @@ func (o *ownerImpl) calcResolvedTs() error {
minCheckpointTs := cfInfo.TargetTs

if len(cfInfo.tables) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after this change when there no tables, the resolve ts can be forward and the checkpoint ts can not be forward?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, in fact if there is no tables, the checkpoint ts is meaningless until cdc meets an add table DDL.

Copy link
Contributor

@july2993 july2993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@july2993 july2993 added LGT1 and removed status/ptal Could you please take a look? labels Dec 9, 2019
@july2993 july2993 requested a review from suzaku December 10, 2019 03:30
Copy link
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zier-one zier-one added LGT2 and removed LGT1 labels Dec 10, 2019
@amyangfei amyangfei merged commit 810cafa into pingcap:master Dec 10, 2019
@amyangfei amyangfei deleted the fix-owner-ts-at-startup branch December 10, 2019 06:16
5kbpers pushed a commit to 5kbpers/ticdc that referenced this pull request Aug 24, 2020
amyangfei added a commit to amyangfei/tiflow that referenced this pull request May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants